feat(biome_glob): add dedicated crate for globs #4609
Conversation
github-actions
bot
added
A-Linter
Conaclos
force-pushed
the
conaclos/biome_glob
branch
from
81e938c
to
799cd3f
Compare
CodSpeed Performance ReportMerging #4609 will not alter performanceComparing Summary
|
Conaclos
force-pushed
the
conaclos/biome_glob
branch
from
799cd3f
to
72c0a2a
Compare
crates/biome_glob/src/lib.rs
Outdated
| @@ -1,31 +1,86 @@ | |||
| use biome_rowan::{TextRange, TextSize}; | |||
| //! biome_glob provides a glob and glob list with exceptions matching. | |||
There was a problem hiding this comment.
| //! biome_glob provides a glob and glob list with exceptions matching. | |
| //! `biome_glob` provides a glob and glob list with exceptions matching. |
| //! let glob = "*.rs".parse::<Glob>().expect("correct glob"); | ||
| //! assert!(glob.is_match("lib.rs")); | ||
| //! assert!(!glob.is_match("src/lib.rs")); |
There was a problem hiding this comment.
This example doesn't show the usage of FromStr::from_str and we should add it.
Also, I would remove the is_match because the previous paragraph didn't mention the is_match function. If you intend the keep is_match in the example, you should explain it in the previous paragraph, and explain why src/lib.rs isn't a match, and why lib.rs is a match.
| //! When a path is expected to be matched against several globs, | ||
| //! you should compile the path into a [CandidatePath]. | ||
| //! [CandidatePath] may speed up matching against several globs. | ||
| //! |
There was a problem hiding this comment.
Let's add a paragraph that explains the example.
| //! | ||
| //! ## Matching against multiple globs and exceptions | ||
| //! | ||
| //! biome_glob supports negated globs, which are particularly useful for encoding exceptions. |
There was a problem hiding this comment.
| //! biome_glob supports negated globs, which are particularly useful for encoding exceptions. | |
| //! biome_glob supports exceptions - or negated globs -, which are particularly useful for encoding exceptions. |
What "encoding expectations" actually mean? We could try to use simpler language.
I would add a small link/reference to git negated globs. What do you think?
crates/biome_glob/src/lib.rs
Outdated
| //! In the following example we accept all files in the `src` dierctory, except the ones ending with the `txt` extension. | ||
| //! |
There was a problem hiding this comment.
It's good that we explain the example, but the example is incorrect 😅 there's no src directory. Also, you should explain what matches_with_exceptions does and how it behaves compared with is_match
| //! - star `*` that matches zero or more characters inside a path segment | ||
| //! - globstar `**` that matches zero or more path segments | ||
| //! - Use `\*` to escape `*` | ||
| //! - `?`, `[`, `]`, `{`, and `}` must be escaped using `\`. | ||
| //! These characters are reserved for future use. | ||
| //! - Use `!` as first character to negate the glob |
There was a problem hiding this comment.
I think we should provide a minimal example for each bullet that we explain
| //! - Use `!` as first character to negate the glob | ||
| //! | ||
| //! A path segment is delimited by path separator `/` or the start/end of the path. | ||
| //! |
There was a problem hiding this comment.
I believe we should have a brief explanation of the features available by this crate
Co-authored-by: Emanuele Stoppa <[email protected]>
Co-authored-by: Emanuele Stoppa <[email protected]>
|
Just a small nit-pick: On a (only partially) related note:
|
|
The idea is to get rid of Pattern. See #4611 for the whole plan, help is very welcome |
Summary
Move
RetsrictedGlobfrombiome_js_analyzeto a dedicated crate.This allows us to use
RetsrictedGloboutsidebiome_js_analyze.Also, I renamed
RetsrictedGlobintoGlob. Possible alternative names areGlobPatternandPattern.I added some documentation for the crate and put optional dependencies behind feature flags.
Test Plan
CI must pass.